chore(cli): generate conversion from cdk.json to cli arguments #32803

kaizencc · 2025-01-09T01:04:46Z

This PR does not change the functionality of the CLI (yet)

It does however articulate a schema for what cdk.json should look like in the future. I'm aware that we honor a slightly different set of rules in cdk.json that is not documented anywhere, and we will have to honor those rules ad-hoc. However, this will hopefully move us towards a strongly-typed future where cdk.json contents mirror CLI argument options.

global options are specified at the base level of cdk.json
command specific options will be prefixed by their command name. NOTE: some options are honored at the base level today. I will have to, in a separate PR, find each of these instances and take care of them but ensuring we still map them to the correct place in CliArguments.

{
  "app": "npx ts-node -P tsconfig.json --prefer-ts-exts src/main.ts",
  "output": "cdk.out",
  "build": "npx projen bundle",
  "watch": {
      "exclude": [
        "README.md",
        "cdk*.json",
        "**/*.d.ts",
        "**/*.js",
        "tsconfig.json",
        "package*.json",
        "yarn.lock",
        "node_modules"
    ]
}

This will turn into the following CliArgument object:

{
  globalOptions: {
    app:  'npx ts-node -P tsconfig.json --prefer-ts-exts src/main.ts',
    output: 'cdk.out',
    build: 'npx projen bundle',
    watch: {
      exclude: [
          "README.md",
          "cdk*.json",
          "**/*.d.ts",
          "**/*.js",
          "tsconfig.json",
          "package*.json",
          "yarn.lock",
          "node_modules",
      ],
   },
};

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

codecov · 2025-01-09T01:23:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.52%. Comparing base (d6013a7) to head (d11171a).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32803   +/-   ##
=======================================
  Coverage   81.52%   81.52%           
=======================================
  Files         222      222           
  Lines       13717    13717           
  Branches     2417     2417           
=======================================
  Hits        11183    11183           
  Misses       2254     2254           
  Partials      280      280

Flag	Coverage Δ
suite.unit	`81.52% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`80.98% <ø> (ø)`
packages/aws-cdk-lib/core	`82.09% <ø> (ø)`

kaizencc · 2025-01-09T04:05:20Z

packages/aws-cdk/lib/cli-arguments.ts

   * The CLI command name
   */
-  readonly _: Command;
+  readonly _?: Command;


needed because cdk.json doesn't have a specific command.

Can we just make it two different interface then?

I don't think that's necessary. CliArguments does/should not do any enforcing. Enforcement comes from sending our CLI input through yargs. Therefore, I think it makes sense that every option on CliArguments is optional. It's just a schema for config options.

Arguably, CliArguments is now a misnomer because it governs both CLI options and cdk.json options. But the goal is to make sure those two are one and the same anyway.

Would it alleviate your concerns if I renamed CliArguments into Arguments?

If it's not CLI anymore, there's no reason to call it _. We only do that because of yargs.

I think we are turning it into structured user input, so maybe something along the lines of UserInput?

I will rename CliArguments to UserInput, and rename _ to command in a separate PR

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

packages/aws-cdk/lib/convert-to-cli-args.ts

mrgrain · 2025-01-09T12:23:13Z

packages/aws-cdk/lib/convert-to-cli-args.ts

+  const cliArguments: CliArguments = {
+    globalOptions,
+    list: listOptions,
+    synthesize: synthesizeOptions,


should synth be the main name?

that can be done but that would have to be a change to config.ts.

What's the impact here? When are we committing to a specific action name in cdk.json? I think we need to make that call before we are committing to it. WDYT about synth vs synthesize though?

i will create a separate PR that changes the default name of synthesize to synth. I believe we've standardized on that, and up until now yargs allows them to work interchangably. Existing configurations should not change, and then we will allow synth as the keyword in cdk.json to provide default configs.

packages/aws-cdk/lib/convert-to-cli-args.ts

mrgrain

Minor feedback.

Co-authored-by: Momo Kornher <[email protected]>

aws-cdk-automation · 2025-01-09T23:11:33Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: d5c3c56
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-01-09T23:12:05Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-01-09T23:15:03Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

wip

7014395

kaizencc requested a review from a team as a code owner January 9, 2025 01:04

aws-cdk-automation requested a review from a team January 9, 2025 01:04

github-actions bot added the p2 label Jan 9, 2025

mergify bot added the contribution/core This is a PR that came from AWS. label Jan 9, 2025

kaizencc changed the title ~~chore(cli): generate schema for cdk.json configuration~~ chore(cli): generate conversion from cdk.json to cli arguments Jan 9, 2025

kaizencc had a problem deploying to test-pipeline January 9, 2025 01:05 — with GitHub Actions Failure

aws-cdk-automation previously requested changes Jan 9, 2025

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Jan 9, 2025

added test

b5217c8

kaizencc had a problem deploying to test-pipeline January 9, 2025 01:26 — with GitHub Actions Failure

kaizencc commented Jan 9, 2025

View reviewed changes

kaizencc and others added 3 commits January 8, 2025 23:08

delete unnecessary

7c22b07

update test

1131b76

Merge branch 'main' into conroy/generateconfig

1d395ae

kaizencc added pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Jan 9, 2025

kaizencc had a problem deploying to test-pipeline January 9, 2025 04:10 — with GitHub Actions Failure

kaizencc commented Jan 9, 2025

View reviewed changes

packages/aws-cdk/lib/convert-to-cli-args.ts Outdated Show resolved Hide resolved

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 9, 2025

mrgrain reviewed Jan 9, 2025

View reviewed changes

packages/aws-cdk/lib/convert-to-cli-args.ts Outdated Show resolved Hide resolved

mrgrain reviewed Jan 9, 2025

View reviewed changes

packages/aws-cdk/lib/convert-to-cli-args.ts Outdated Show resolved Hide resolved

mrgrain requested changes Jan 9, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 9, 2025

kaizencc and others added 2 commits January 9, 2025 11:21

Update packages/aws-cdk/lib/convert-to-cli-args.ts

4ff6f81

Co-authored-by: Momo Kornher <[email protected]>

Merge branch 'main' into conroy/generateconfig

0539f47

kaizencc had a problem deploying to test-pipeline January 9, 2025 16:22 — with GitHub Actions Failure

rename proeprty to config

e2cb975

kaizencc had a problem deploying to test-pipeline January 9, 2025 18:09 — with GitHub Actions Failure

add comment explaining why we ignore aliases in cdk.json

dd882a3

kaizencc had a problem deploying to test-pipeline January 9, 2025 18:31 — with GitHub Actions Failure

mrgrain added the pr/do-not-merge This PR should not be merged at this time. label Jan 9, 2025

mrgrain approved these changes Jan 9, 2025

View reviewed changes

kaizencc removed the pr/do-not-merge This PR should not be merged at this time. label Jan 9, 2025

Merge branch 'main' into conroy/generateconfig

d11171a

kaizencc had a problem deploying to test-pipeline January 9, 2025 22:06 — with GitHub Actions Failure

empty commit

d5c3c56

kaizencc had a problem deploying to test-pipeline January 9, 2025 22:41 — with GitHub Actions Failure

kaizencc merged commit b670ba8 into main Jan 9, 2025
10 of 11 checks passed

kaizencc deleted the conroy/generateconfig branch January 9, 2025 23:14

github-actions bot locked as resolved and limited conversation to collaborators Jan 9, 2025

chore(cli): generate conversion from cdk.json to cli arguments #32803

chore(cli): generate conversion from cdk.json to cli arguments #32803

Uh oh!

Conversation

kaizencc commented Jan 9, 2025

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaizencc Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mrgrain left a comment

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Jan 9, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Jan 9, 2025

Uh oh!

Uh oh!

github-actions bot commented Jan 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 9, 2025 •

edited

Loading

kaizencc Jan 9, 2025 •

edited

Loading